Skip to content

feat(pluginutils): add staticBlock(es2022) support#2000

Open
linsk1998 wants to merge 4 commits into
rollup:masterfrom
linsk1998:master
Open

feat(pluginutils): add staticBlock(es2022) support#2000
linsk1998 wants to merge 4 commits into
rollup:masterfrom
linsk1998:master

Conversation

@linsk1998
Copy link
Copy Markdown
Contributor

@linsk1998 linsk1998 commented May 18, 2026

Rollup Plugin Name: pluginutils

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.

List any relevant issue numbers:

Description

  1. Static blocks are a new syntax feature introduced in ES2022;
  2. Static blocks run in a function scope;
  3. The pluginutils package currently lacks support for this syntax;
  4. Missing support causes parsing failures for code containing class static blocks, e.g.:
console.log(foo); // The inject plugin incorrectly identifies `foo` as a global variable
class Bar {
    static {
        var foo = 1; // Declared in static block (function-scoped)
    }
}

@linsk1998 linsk1998 requested a review from shellscape as a code owner May 18, 2026 09:19
@shellscape
Copy link
Copy Markdown
Collaborator

shellscape commented May 29, 2026

@CharlieHelps please do a thorough review of this PR to determine if it's ready to merge. Look at the diff, reviews, comments, and referenced issues/docs/links to build a complete understanding. Then respond with a comment that includes a list of MUST fix items that must be fixed before the the PR is merged, and a list of IMPROVEMENTS that shouldn't block the merge but may be quick improvements or things that could be addressed in a follow up PR. The MUST fix items should explain exactly what's wrong and what needs to change to resolve the problem. Both should be numbered lists and the IMPROVEMENTS list should continue counting up from the MUST list. It's OK if there are no IMPROVEMENTS or MUST fix items, you can just omit the lists.

charliecreates[bot]
charliecreates Bot reviewed May 29, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@charliecreates charliecreates Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking feedback (1)
  1. Add a var regression assertion for static-block scoping — packages/pluginutils/test/attachScopes.ts#L76-L96
    The implementation change is specifically about giving StaticBlock function-like scope behavior (block: false), which mainly affects var declarations. The new test currently uses let, so it would not catch a regression where static blocks are treated as plain block scope. Please add a var case (ideally including var inside a nested block) and assert it is contained by staticBlock.scope but not the top-level scope.

If you want me to push fixes, reply with item numbers (for example: please fix 1).

@charliecreates
Copy link
Copy Markdown
Contributor

charliecreates Bot commented May 29, 2026

@shellscape I completed a thorough review of this PR (diff, existing discussion/reviews, CI status, and relevant docs).

I do not see any merge-blocking issues. From a code-quality and correctness standpoint, this looks ready to merge.

IMPROVEMENTS

  1. Add a var-based regression case for static blocks in packages/pluginutils/test/attachScopes.ts (ideally including a nested-block var) and assert it is captured by staticBlock.scope but not top-level scope. This would lock in the intended function-like scoping behavior for static blocks and reduce risk of future regressions.

@shellscape
Copy link
Copy Markdown
Collaborator

shellscape commented May 29, 2026

@linsk1998 I think it's worth adding the test that Charlie mentioned. seems very prudent. Please do so and we'll merge this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@charliecreates charliecreates[bot] charliecreates[bot] left review comments

@shellscape shellscape Awaiting requested review from shellscape shellscape is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@linsk1998 @shellscape